Skip to content

Conversation

@jakeegan
Copy link
Member

@jakeegan jakeegan commented Jun 18, 2025

On 64-bit AIX, set allocator size to 256G and set beginning to 0x0a00000000000000.

Issue: #138916

@jakeegan jakeegan marked this pull request as ready for review June 19, 2025 13:18
@jakeegan jakeegan requested a review from vitalybuka June 19, 2025 13:18
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Jake Egan (jakeegan)

Changes

On AIX, set allocator size to 256G and, on 64-bit, set beginning to 0x0a00000000000000.

Issue: #138916


Full diff: https://github.com/llvm/llvm-project/pull/144784.diff

3 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_allocator.h (+4)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h (+5)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h (+1)
diff --git a/compiler-rt/lib/asan/asan_allocator.h b/compiler-rt/lib/asan/asan_allocator.h
index db8dc3bebfc62..6fb533423580d 100644
--- a/compiler-rt/lib/asan/asan_allocator.h
+++ b/compiler-rt/lib/asan/asan_allocator.h
@@ -197,7 +197,11 @@ const uptr kAllocatorSpace = ~(uptr)0;
 #    endif  // SANITIZER_APPLE
 
 #    if defined(__powerpc64__)
+#      if SANITIZER_AIX
+const uptr kAllocatorSize = 1ULL << 38;  // 256G.
+#      else
 const uptr kAllocatorSize  =  0x20000000000ULL;  // 2T.
+#      endif
 typedef DefaultSizeClassMap SizeClassMap;
 #    elif defined(__aarch64__) && SANITIZER_ANDROID
 // Android needs to support 39, 42 and 48 bit VMA.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h
index 62523c7ae187c..e5b912d70e61e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_internal.h
@@ -23,7 +23,12 @@ namespace __sanitizer {
 typedef CompactSizeClassMap InternalSizeClassMap;
 
 struct AP32 {
+// For AIX 64-bit, the mmap begin is at address 0x0a00000000000000ULL.
+#if SANITIZER_AIX && SANITIZER_WORDSIZE == 64
+  static const uptr kSpaceBeg = 0x0a00000000000000ULL;
+#else
   static const uptr kSpaceBeg = 0;
+#endif
   static const u64 kSpaceSize = SANITIZER_MMAP_RANGE_SIZE;
   static const uptr kMetadataSize = 0;
   typedef InternalSizeClassMap SizeClassMap;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h
index 602b197c42ae3..0faf9b3c151a4 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h
@@ -288,6 +288,7 @@ class SizeClassAllocator32 {
   uptr ComputeRegionId(uptr mem) const {
     if (SANITIZER_SIGN_EXTENDED_ADDRESSES)
       mem &= (kSpaceSize - 1);
+    mem -= kSpaceBeg;
     const uptr res = mem >> kRegionSizeLog;
     CHECK_LT(res, kNumPossibleRegions);
     return res;

@github-actions
Copy link

github-actions bot commented Jul 10, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@jakeegan jakeegan requested a review from vitalybuka July 10, 2025 04:49
@hubert-reinterpretcast hubert-reinterpretcast dismissed their stale review July 10, 2025 22:13

For some reason only saw changes to one file.

Copy link
Collaborator

@hubert-reinterpretcast hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no specific additional concerns, but would like to defer approval to another reviewer.

Co-authored-by: Hubert Tong <[email protected]>
@jakeegan
Copy link
Member Author

Addressed comments.

uptr ComputeRegionId(uptr mem) const {
if (SANITIZER_SIGN_EXTENDED_ADDRESSES)
mem &= (kSpaceSize - 1);
mem -= kSpaceBeg;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look NFC for other platforms?

also seems mem &= is to achieve the similar thing. Combining both seems wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kSpaceBeg is zero for other platforms, so it's NFC.

&= doesn't always give the same result as -= if mem is high enough. For example, if mem = 0x0c00 and kSpaceBeg = 0x0a00.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants